feat: probe Sui providers by genesis checkpoint digest - #4070

Pull request overview

Wires Sui into the network-fingerprint probe by implementing NetworkFingerprintInspector for SuiInspector, reading the base58 genesis-checkpoint digest out of GetServiceInfo.chain_id. The old classify_status free function is replaced by the ClassifyRpcOutcome blanket impl over Result<T, tonic::Status> introduced in #4069, so NotFound now resolves through the response type's HasAbsenceMeaning (transaction -> TransactionNotFound, service info -> RpcRequestRejected) rather than being unconditionally a missing transaction. DeadlineExceeded is split out to Timeout, matching what the Aptos classifier already does.

The probe path is still library-only (probe_all_providers has no production caller yet, see crates/foreign-chain-health-check/src/lib.rs:43), and Timeout/RpcRequestFailed are indistinguishable to FanOut::extract (both are is_transient()), so the signing path is unaffected - as the PR body claims.

Changes:

• SuiInspector gains network_fingerprint (identity canonical_fingerprint, since base58 has one spelling) and a ForeignChain::Sui arm in probe_all_providers; the TODO(#4003) is retired.
• classify_status -> ClassifyRpcOutcome for Result<T, Status>, with HasAbsenceMeaning for GetTransactionResponse / GetServiceInfoResponse, and DeadlineExceeded remapped to Timeout.
• Test scaffolding: a real tonic gRPC fake (FakeSuiLedger/FakeSuiServer) in the health-check crate, since httpmock cannot serve gRPC; MockSuiClient now arms both RPCs; a live #[ignore]d fingerprint check mirroring the Aptos one.
• Docs: Sui added to the probe table, prose rewritten around "every chain with an inspector is probed".

Reviewed changes

Findings

Blocking (must fix before merge):

• docs/foreign-chain-transactions.md:551 and :722-723 - both new sentences name ton as a chain that "has no inspector, so [it] ignore[s] expected_network_fingerprint" / "report[s] ProbeNotImplemented whether the field is set or not". ton has no slot in the node's config at all (crates/node-config/src/foreign_chains.rs:17-43) and is never yielded by all_configured_chains (:164-182), so it can never be configured, probed, or produce a report row - the ProbeNotImplemented claim is false for it, and it contradicts the unchanged sentence at :713 ("solana and ethereum are configurable but absent from the table"). Same inaccuracy in the code comment at crates/foreign-chain-health-check/src/probe.rs:140. Either drop ton from all three spots, or state that it is a contract-side chain with no node config.

Non-blocking (nits, follow-ups, suggestions):

• crates/foreign-chain-health-check/src/probe.rs:141 - with Sui landed, the only variants reaching _ => ProbeNotImplemented are Solana and Ethereum, yet the docs now promise "every chain with an inspector is probed". The wildcard silently breaks that promise for the next chain added to ForeignChain. Spelling the arm out (ForeignChain::Solana | ForeignChain::Ethereum | ForeignChain::Ton =>) turns that into a compile error instead of an unprobed row on the dashboard.
• crates/foreign-chain-inspector/src/sui/inspector.rs:137 - the impl is keyed on tonic::Status, not on anything Sui-specific, but lives in the Sui module. A future gRPC-based chain silently inherits Sui's mapping and, by coherence, cannot override it. Consider moving it beside the trait in lib.rs so its reach is visible at the definition site.
• crates/foreign-chain-inspector/src/sui/inspector.rs:148-151 - worth confirming (I could not check tonic's source from this checkout): tonic derives a Status from transport-level std::io::Errors, and if that conversion maps ErrorKind::NotFound to Code::NotFound, a purely local transport failure would be classified as TransactionNotFound - a non-transient chain verdict in FanOut::extract, not an availability blip. Pre-existing rather than introduced here, but this PR is the one refining exactly this axis, so it seems the right place to confirm the assumption that Code::NotFound only ever comes from the server.
• crates/foreign-chain-inspector/tests/sui_inspector.rs:30 and :43 - the un-armed RPC now answers Status::unimplemented(...), which the code under test classifies as RpcRequestRejected. A test that reaches the wrong RPC therefore fails (or passes) on a plausible-looking assertion instead of panicking loudly; get_checkpoint at :59 still uses unimplemented!(), so the mock now mixes both conventions. Option<Result<...>> plus .expect("test did not arm get_service_info") would keep the loud failure.
• crates/foreign-chain-inspector/tests/sui_inspector.rs:20 - "answering the one call a test arms, and refusing the other" does not describe status(), which arms both with the same Status.

⚠️ Issues found

@claude review

Pull request overview

Third step of the #4003 probe stack: SuiInspector gains NetworkFingerprintInspector, reading the base58 genesis-checkpoint digest out of GetServiceInfo.chain_id, and probe_all_providers gains a ForeignChain::Sui arm. The old classify_status free function is replaced by the ClassifyRpcOutcome blanket impl over Result<T, tonic::Status> introduced in #4069, so NotFound now resolves through the response type's HasAbsenceMeaning (transaction → TransactionNotFound, service info → RpcRequestRejected) rather than being unconditionally a missing transaction. DeadlineExceeded is split out to Timeout.

The blocking documentation inaccuracy from the previous round is fixed: the prose and the code comment now name only solana and ethereum, which matches both all_configured_chains (crates/node-config/src/foreign_chains.rs:164-182) and the set of inspector modules that actually exist. MockSuiClient now panics on an unarmed RPC instead of answering a plausible-looking Status::unimplemented, and its doc comment describes what it does.

probe_all_providers still has no production caller (crates/foreign-chain-health-check/src/lib.rs:43), and Timeout/RpcRequestFailed are indistinguishable to FanOut::extract (both is_transient()), so the signing path is unaffected — as the PR body claims.

Changes:

• NetworkFingerprintInspector for SuiInspector, with an identity canonical_fingerprint (base58 has one spelling), plus the ForeignChain::Sui arm in probe_all_providers; TODO(#4003) retired.
• classify_status → impl<T: HasAbsenceMeaning> ClassifyRpcOutcome for Result<T, Status>, with HasAbsenceMeaning for GetTransactionResponse / GetServiceInfoResponse, and DeadlineExceeded remapped to Timeout.
• Test scaffolding: a real tonic gRPC fake (FakeSuiLedger/FakeSuiServer) in the health-check crate, since httpmock cannot serve gRPC; six probe_all_providers verdict tests; MockSuiClient arms each RPC independently; a live #[ignore]d fingerprint check mirroring the Aptos one.
• Docs: Sui row in the probe table, prose rewritten around "every chain with an inspector is probed".

Reviewed changes

Findings

Non-blocking (nits, follow-ups, suggestions):

• crates/foreign-chain-inspector/src/sui/inspector.rs:157 / crates/foreign-chain-health-check/src/probe.rs:142 — the case this arm exists to serve (a provider that stalls) is decided by a race between three deadlines set to the same value. prepare_sui(provider, timeout) gives the client a per-request gRPC deadline (request.set_timeout, crates/foreign-chain-rpc-interfaces/src/sui.rs:95-99) equal to timeout_of(chain_config), which is the same duration FanOut::network_fingerprints passes to its own tokio::time::timeout (crates/foreign-chain-inspector/src/lib.rs:253). So a stall can surface as: the server honouring grpc-timeout → DEADLINE_EXCEEDED → TimedOut; tonic's own client-side timeout → TimedOut only if tonic reports it as DEADLINE_EXCEEDED; or the probe's tokio::time::timeout → Timeout → TimedOut. I could not check tonic's source from this checkout, but its GrpcTimeout layer has historically produced Status::cancelled("Timeout expired"), which lands in the RpcRequestFailed arm → ProviderStatus::Unreachable — exactly the verdict the change sets out to remove, and what the comment on :156 promises it will not be. Worth confirming; if it is Cancelled, the cheapest fix is to let the probe own the deadline (pass a generous or no per-request timeout to prepare_sui from the probe arm) rather than to also map Cancelled, which legitimately means "caller cancelled" in other contexts.
  • Related coverage gap: probe_all_providers__should_report_a_slow_sui_provider_as_timed_out pins the mapping — the fake answers DEADLINE_EXCEEDED immediately — not the scenario the name describes, and the stalled-call test was dropped in 63c9fc6. Starknet still has the end-to-end version (probe_all_providers__should_report_a_provider_that_does_not_answer_in_time, probe.rs:599); Sui now has nothing equivalent.
• crates/foreign-chain-health-check/src/probe.rs:995 — FakeSuiLedger implements only get_service_info, so the remaining LedgerService RPCs necessarily fall through to tonic's generated default stubs and answer Status::unimplemented. Through classified() that is RpcRequestRejected → ProviderStatus::RequestRejected: a plausible-looking verdict, which is the same soft failure f3ae667 just removed from MockSuiClient. If the probe ever calls a second RPC, these tests assert a wrong status instead of failing loudly. Overriding the RPCs the fake should never receive with unreachable!() keeps the two fakes consistent.
• Unchanged from the previous round, restated only so they are not lost: the _ => wildcard at probe.rs:147 (now that the docs promise "every chain with an inspector is probed", spelling out ForeignChain::Solana | ForeignChain::Ethereum turns the next chain added to ForeignChain into a compile error rather than a silently unprobed dashboard row); the ClassifyRpcOutcome for Result<T, Status> impl living in the Sui module while being keyed only on tonic::Status; and confirming that Code::NotFound can only originate server-side, never from a local transport error.

✅ Approved